Security: Restrict direct attachment access and upload file types #681
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request strengthens the security of attachment handling in two ways.
First, it hardens the
attachments/directory by denying all direct HTTP access via.htaccess. Attachments are therefore no longer accessible via direct URLs underattachments/and must be served through the application layer (AttachmentsUI), where authentication and authorization checks already exist.Second, it introduces a server-side whitelist of allowed file extensions in
AttachmentCreator::createFromUpload(), so only a predefined set of document and image formats can be uploaded. This reduces the risk of storing and serving unexpected or potentially dangerous file types while still supporting common recruiting-related formats.As part of this change, internal UI references were updated to consistently use the existing retrieval URLs instead of direct
attachments/paths, ensuring that downloads and inline previews (such as candidate photos) continue to work with direct access denied.The unused legacy attachment download precheck assets (
js/attachment.jsandajax/getAttachmentLocal.php) and their template includes were removed.Motivation
Securing access to stored attachments
Before this change, files stored under
attachments/could be accessed via direct URLs for specific extensions, which meant that knowledge of the path alone could be sufficient to retrieve a file without passing through OpenCATS authentication and authorization logic.While the paths are not trivial to guess, this does not align with the principle of defense in depth. By denying all direct HTTP access to
attachments/and relying exclusively onAttachmentsUI::getAttachment()for retrieval, attachment access consistently flows through the existing permission checks and is no longer dependent on any URL obfuscation details or web server behaviors.Restricting allowed upload types
Previously there was no server-side restriction on which file types could be uploaded through the attachment upload flow. Any extension that passed basic size and upload error checks was accepted. This increased the risk of storing file types that are unnecessary for typical recruiting workflows and could introduce security issues, especially if they are rendered inline by the browser.
Introducing a server-side whitelist based on common document and image formats narrows the attack surface and prevents the upload of unexpected or potentially dangerous file types. In particular, excluding
htmlhelps mitigate stored cross-site scripting (XSS) risks where uploaded HTML could otherwise be served under the OpenCATS origin and executed in a user's session.